-
Notifications
You must be signed in to change notification settings - Fork 13.3k
'for' loop hygiene, etc. #15184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
'for' loop hygiene, etc. #15184
Conversation
fld.cx.expr_match(span, discrim, vec!(arm)) | ||
// why these clone()'s everywhere? I guess I'll follow the pattern.... | ||
let match_expr = fld.cx.expr_match(span, discrim, vec!(arm)); | ||
fld.fold_expr(match_expr).clone() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this compile without the .clone
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll give it a try. I worry that there's some hidden mutability that makes the clone() necessary and that the tests won't catch. Perhaps I should just have faith in the type system. If this one can go, I think there are a lot of other places where it could go, as well....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, since it's widespread sounds like that could be a separate patch to avoid blocking this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I'm going to leave it alone for now.
r=me if clone removed |
@bors: retry |
…ents It turns out that bindings introduced by 'for' loops were not treated hygienically. The fix for this is to make the 'for' expansion more like a macro; rather than expanding sub-pieces and then assembling them, we need to rewrite the for and then call expand again on the whole thing. This PR includes a test and the fix. It also contains a number of other things: - unit tests for other forms of hygiene (currently ignored) - a fix for the isaac.rs macro that (it turned out) was relying on capturing - other miscellaneous cleanup and comments
Disable mir interpreter for targets with different pointer size from host fix rust-lang#15182
It turns out that bindings introduced by 'for' loops were not treated hygienically. The fix for this is to make the 'for' expansion more like a macro; rather than expanding sub-pieces and then assembling them, we need to rewrite the for and then call expand again on the whole thing.
This PR includes a test and the fix.
It also contains a number of other things: